Skip to content

Add Floyd-Warshall pathfinding algorithm#116

Open
jshyun912 wants to merge 20 commits into
utilForever:mainfrom
jshyun912:feat/83-fw
Open

Add Floyd-Warshall pathfinding algorithm#116
jshyun912 wants to merge 20 commits into
utilForever:mainfrom
jshyun912:feat/83-fw

Conversation

@jshyun912
Copy link
Copy Markdown
Contributor

@jshyun912 jshyun912 commented May 31, 2026

What

Implemented Floyd-WarshallMap with tests and examples.

Why

Floyd-Warshall can provides all-pairs shortest path mapping, enabling efficient path queries between any two points after preprocessing.

close #83

Checklist

Required

  • cargo check --all passes
  • cargo fmt --all -- --check passes
  • cargo clippy --workspace --all-targets -- -D warnings -A clippy::multiple-crate-versions passes
  • cargo test --all passes
  • I linked the related issue (for example: Closes #123)

Functional Validation

  • Behavior related to this change was verified locally (if applicable)
  • Rendering/backend behavior was verified when runtime code changed (if applicable)
  • Algorithm behavior (pathfinding/FOV/noise/random) was verified when affected (if applicable)
  • I added or updated tests for changed behavior (if applicable)

Configuration & Docs

  • User-facing docs were updated (README.md, ARCHITECTURE.md, or relevant manual pages, if applicable)
  • New dependencies/configuration are documented (if applicable)
  • No sensitive values or credentials were introduced

If Applicable

  • Security impact considered (run cargo audit locally if needed)
  • Breaking behavior changes are clearly described in this PR

Summary by CodeRabbit

  • New Features

    • Added Floyd–Warshall pathfinding as a supported algorithm and made it available from the crate’s convenience imports.
  • Documentation

    • Updated architecture docs and README with a Floyd–Warshall section, usage snippet, and performance/memory warnings.
  • Examples

    • Added a new example that visualizes Floyd–Warshall all-pairs path mapping on a tile map.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a FloydWarshallMap with a flattened all-pairs distance matrix and build routine, traversal helpers (lowest/highest exit), an example visualization with console helpers and Map/trait implementations, re-exports the module in the crate prelude, and updates README and ARCHITECTURE.md to document Floyd–Warshall support.

Changes

Floyd-Warshall Pathfinding

Layer / File(s) Summary
FloydWarshallMap type, build, and traversal helpers
bracket-pathfinding/src/floyd_warshall.rs
Adds FloydWarshallMap { pub depth_map: Vec<f32>, size_x, size_y, max_depth }, pub fn new(...) that allocates and invokes build, idx_helper, build implementing Floyd–Warshall relaxation, find_lowest_exit and find_highest_exit, plus unit tests.
Crate prelude export
bracket-pathfinding/src/lib.rs
Re-exports the floyd_warshall module through the crate prelude, exposing Floyd–Warshall symbols via bracket_pathfinding::prelude::*.
Example: map, traits, and console visualization
bracket-pathfinding/examples/floyd_warshall/common.rs, bracket-pathfinding/examples/floyd_warshall/main.rs
Adds console helper functions, map constants and Map struct with Map::new() placing deterministic walls, BaseMap and Algorithm2D implementations (8-direction exits; cardinal cost 1.0, diagonal 1.4), and a main example that builds FloydWarshallMap and renders per-tile colored output from depth_map.
Docs and ARCHITECTURE updates
ARCHITECTURE.md, bracket-pathfinding/README.md
Adds a "Floyd-Warshall Mapping" section describing FloydWarshallMap::new usage and performance/memory warning; ARCHITECTURE.md now lists Floyd–Warshall support and README examples list includes a floyd warshall example.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant FloydWarshallMap
  participant BaseMap
  Caller->>FloydWarshallMap: new(size_x, size_y, map, max_depth)
  FloydWarshallMap->>BaseMap: get_available_exits(pos) (seed direct edges)
  FloydWarshallMap->>FloydWarshallMap: run Floyd–Warshall relaxation (populate depth_map)
  Caller->>FloydWarshallMap: find_lowest_exit(position, map) / find_highest_exit(...)
  FloydWarshallMap->>BaseMap: get_available_exits(position) (enumerate candidate exits)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped the grid where distances spread,
I traced each route where bytes were led,
A matrix filled with every way,
The rabbit paints the shortest sway,
Bright colors bloom where paths are sped.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add Floyd-Warshall pathfinding algorithm' directly and clearly describes the main change—implementing a new pathfinding algorithm. It is concise, specific, and accurately reflects the primary objective of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
bracket-pathfinding/src/floyd_warshall.rs (1)

41-126: ⚡ Quick win

Prefer &self/&mut self receivers over passing fm explicitly.

build, find_lowest_exit, and find_highest_exit take the map as a fm: &FloydWarshallMap parameter rather than using a method receiver. This is non-idiomatic; method-call syntax (f.build(map), fm.find_lowest_exit(pos, map)) reads more naturally and matches the rest of the crate's style.

♻️ Proposed receiver-based signatures
-    pub fn build(fm: &mut FloydWarshallMap, map: &dyn BaseMap) {
-        let mapsize: usize = fm.size_x * fm.size_y;
+    pub fn build(&mut self, map: &dyn BaseMap) {
+        let mapsize: usize = self.size_x * self.size_y;

Update new accordingly:

-        FloydWarshallMap::build(&mut f, map);
+        f.build(map);

Apply the same &self transformation to find_lowest_exit and find_highest_exit.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bracket-pathfinding/src/floyd_warshall.rs` around lines 41 - 126, Change the
free functions build, find_lowest_exit, and find_highest_exit into inherent
methods on FloydWarshallMap using &mut self for build and &self for the two
finders (so callers can use fm.build(map), fm.find_lowest_exit(pos, map),
fm.find_highest_exit(pos, map)); update signatures and bodies to remove the
explicit fm parameter and reference fields directly (e.g., self.size_x,
self.size_y, self.idx_helper(...), self.depth_map, self.max_depth) and adjust
the new method receiver in any callers and in new() if it currently calls build
as a free function. Ensure mutability for build so it writes to self.depth_map
and keep find_lowest_exit/find_highest_exit as non-mutating &self methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bracket-pathfinding/examples/floyd_warshall/common.rs`:
- Around line 105-110: The code duplicates the south-west diagonal check by
calling valid_exit(location, Point::new(-1, 1)) twice, which omits the
south-east diagonal; update the second occurrence to call valid_exit(location,
Point::new(1, 1)) so the exits vector gets the southeast neighbor (push the same
weight 1.4), i.e., change the duplicate Point::new(-1, 1) to Point::new(1, 1) in
the block that builds exits using valid_exit and pushes into exits.

In `@bracket-pathfinding/examples/floyd_warshall/main.rs`:
- Line 23: The reachability check is using the wrong flattened index: replace
the use of flow_map.depth_map[idx] with flow_map.depth_map[depth_map_idx] so the
test checks the start_idx → idx entry (the same entry the gradient rendering
uses); update the if condition that currently reads `if flow_map.depth_map[idx]
< f32::MAX` to use `depth_map_idx` (and ensure depth_map_idx is computed as
`start_idx * mapsize + idx` or the existing calculation) so reachability and
rendered depth are consistent.

In `@bracket-pathfinding/README.md`:
- Line 108: The README heading "Floyd-Washall Mapping" contains a typo; update
the heading text to "Floyd-Warshall Mapping" by replacing the string
"Floyd-Washall" with "Floyd-Warshall" in the README (the section title line that
currently reads "## Floyd-Washall Mapping").
- Line 141: The README line describing the example `floyd_warshall` is
incorrect: it says it demonstrates "Dijkstra mapping" but the example implements
the Floyd–Warshall algorithm (all-pairs shortest paths). Update the bullet for
`floyd_warshall` to accurately describe what the example shows (e.g.,
"Floyd–Warshall (cargo run --example floyd_warshall), demonstrating the
Floyd–Warshall all-pairs shortest-path algorithm" or similar), replacing the
Dijkstra wording; reference the `floyd_warshall` example name in your edit.

In `@bracket-pathfinding/src/floyd_warshall.rs`:
- Around line 21-22: The current conversion uses size_x.try_into().ok().unwrap()
and size_y.try_into().ok().unwrap(), which hides conversion errors and produces
an opaque panic; replace these with explicit error handling such as
size_x.try_into().expect("failed to convert size_x to usize") and
size_y.try_into().expect("failed to convert size_y to usize") or propagate the
Result from the function so failures carry context—update the bindings sz_x and
sz_y accordingly to use expect (or ? propagation) instead of .ok().unwrap().

---

Nitpick comments:
In `@bracket-pathfinding/src/floyd_warshall.rs`:
- Around line 41-126: Change the free functions build, find_lowest_exit, and
find_highest_exit into inherent methods on FloydWarshallMap using &mut self for
build and &self for the two finders (so callers can use fm.build(map),
fm.find_lowest_exit(pos, map), fm.find_highest_exit(pos, map)); update
signatures and bodies to remove the explicit fm parameter and reference fields
directly (e.g., self.size_x, self.size_y, self.idx_helper(...), self.depth_map,
self.max_depth) and adjust the new method receiver in any callers and in new()
if it currently calls build as a free function. Ensure mutability for build so
it writes to self.depth_map and keep find_lowest_exit/find_highest_exit as
non-mutating &self methods.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c985c7bc-0d8f-41f8-b1fc-f0cb2f09f16e

📥 Commits

Reviewing files that changed from the base of the PR and between fe7bf77 and 4a4513f.

📒 Files selected for processing (6)
  • ARCHITECTURE.md
  • bracket-pathfinding/README.md
  • bracket-pathfinding/examples/floyd_warshall/common.rs
  • bracket-pathfinding/examples/floyd_warshall/main.rs
  • bracket-pathfinding/src/floyd_warshall.rs
  • bracket-pathfinding/src/lib.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: 🕸️ WASM Build
  • GitHub Check: ✅ Check (macos-latest)
  • GitHub Check: ✅ Check (windows-latest)
  • GitHub Check: ✅ Check (ubuntu-latest)
  • GitHub Check: 🧪 Test Suite
  • GitHub Check: 📦 Unused Crates
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format all Rust code using cargo fmt --all
Fix all cargo clippy warnings — the CI enforces -D warnings
Add tests for new functionality in the relevant module; for split domains, prefer colocated tests.rs

Files:

  • bracket-pathfinding/src/lib.rs
  • bracket-pathfinding/examples/floyd_warshall/main.rs
  • bracket-pathfinding/src/floyd_warshall.rs
  • bracket-pathfinding/examples/floyd_warshall/common.rs
🔇 Additional comments (4)
ARCHITECTURE.md (1)

73-73: LGTM!

bracket-pathfinding/src/floyd_warshall.rs (1)

145-171: LGTM!

bracket-pathfinding/src/lib.rs (1)

6-12: LGTM!

bracket-pathfinding/examples/floyd_warshall/common.rs (1)

40-60: ⚡ Quick win

Prevent clippy::new_without_default from failing -D warnings in the Floyd-Warshall example

clippy::new_without_default will fire for a public pub fn new() -> Self type when that type lacks a Default impl and the examples are included in the clippy run (e.g., via cargo clippy --all-targets). Adding Default to this example’s Map avoids the lint.

🛠️ Add a Default impl
+impl Default for Map {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl Map {
     pub fn new() -> Self {

Comment thread bracket-pathfinding/examples/floyd_warshall/common.rs Outdated
Comment thread bracket-pathfinding/examples/floyd_warshall/main.rs Outdated
Comment thread bracket-pathfinding/README.md Outdated
Comment thread bracket-pathfinding/README.md Outdated
Comment on lines +21 to +22
let sz_x: usize = size_x.try_into().ok().unwrap();
let sz_y: usize = size_y.try_into().ok().unwrap();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

.ok().unwrap() discards the conversion error and panics opaquely.

If try_into fails, the resulting panic has no context. Prefer .expect(...) (or propagate) for a clearer failure message.

🛠️ Suggested change
-        let sz_x: usize = size_x.try_into().ok().unwrap();
-        let sz_y: usize = size_y.try_into().ok().unwrap();
+        let sz_x: usize = size_x.try_into().ok().expect("size_x must fit in usize");
+        let sz_y: usize = size_y.try_into().ok().expect("size_y must fit in usize");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let sz_x: usize = size_x.try_into().ok().unwrap();
let sz_y: usize = size_y.try_into().ok().unwrap();
let sz_x: usize = size_x.try_into().ok().expect("size_x must fit in usize");
let sz_y: usize = size_y.try_into().ok().expect("size_y must fit in usize");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bracket-pathfinding/src/floyd_warshall.rs` around lines 21 - 22, The current
conversion uses size_x.try_into().ok().unwrap() and
size_y.try_into().ok().unwrap(), which hides conversion errors and produces an
opaque panic; replace these with explicit error handling such as
size_x.try_into().expect("failed to convert size_x to usize") and
size_y.try_into().expect("failed to convert size_y to usize") or propagate the
Result from the function so failures carry context—update the bindings sz_x and
sz_y accordingly to use expect (or ? propagation) instead of .ok().unwrap().

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bracket-pathfinding/src/floyd_warshall.rs`:
- Around line 17-31: Add colocated unit tests for FloydWarshallMap::new in a
tests.rs inside the same module: test that constructing with small sizes (use
TryInto-able integers) produces FloydWarshallMap with size_x and size_y set
correctly and map.len() == size_x * size_y, that every entry in the map vector
equals f32::MAX initially, and a simple sanity test using a minimal BaseMap stub
where reachable adjacency is known to assert default max_depth is stored in
max_depth and no paths are computed by new (i.e., map entries remain f32::MAX).
Reference FloydWarshallMap::new and the struct fields map, size_x, size_y, and
max_depth when writing the assertions.
- Around line 23-31: The constructor is allocating sz_x * sz_y (one distance per
tile) and skips computation; change allocation to tile_count * tile_count using
checked multiplication (e.g., let tile_count =
sz_x.checked_mul(sz_y).expect("..."); let total =
tile_count.checked_mul(tile_count).expect("...")) to avoid overflow, initialize
map Vec<f32> with total entries, then call FloydWarshallMap::build(&mut f,
starts, map) (uncomment/restore that call) before returning the FloydWarshallMap
so the all-pairs distances are computed; keep references to FloydWarshallMap,
build, sz_x, sz_y, starts, map, and max_depth to locate the edits.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6bdc2124-8fa1-43c5-a2a3-76c1659a19f7

📥 Commits

Reviewing files that changed from the base of the PR and between 4a4513f and 34c8d22.

📒 Files selected for processing (1)
  • bracket-pathfinding/src/floyd_warshall.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ✅ Check (windows-latest)
  • GitHub Check: 📦 Unused Crates
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format all Rust code using cargo fmt --all
Fix all cargo clippy warnings — the CI enforces -D warnings
Add tests for new functionality in the relevant module; for split domains, prefer colocated tests.rs

Files:

  • bracket-pathfinding/src/floyd_warshall.rs
🔇 Additional comments (1)
bracket-pathfinding/src/floyd_warshall.rs (1)

21-22: try_into().ok().unwrap() still hides conversion context.

This was already flagged in previous review feedback and remains unresolved.

Comment thread bracket-pathfinding/src/floyd_warshall.rs
Comment thread bracket-pathfinding/src/floyd_warshall.rs Outdated
@jshyun912 jshyun912 closed this Jun 1, 2026
@jshyun912 jshyun912 reopened this Jun 1, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bracket-pathfinding/src/floyd_warshall.rs`:
- Around line 74-96: The helpers currently pick exits by local edge cost; update
both find_lowest_exit and find_highest_exit to accept an additional
target/source index parameter (e.g., dest: usize) and use fm.idx_helper(exit,
dest) to look up the distance in fm.depth_map when ranking candidates instead of
fm.idx_helper(position, exit). Specifically, change signatures of
find_lowest_exit and find_highest_exit to include the destination/source,
iterate exits from map.get_available_exits(position), compute pos =
fm.idx_helper(exit, dest) and compare fm.depth_map[pos] to choose the min/max
exit, and add a regression test with asymmetric edge weights exercising both
helpers so the choice depends on distance-to-target rather than the immediate
edge cost.
- Around line 41-69: The build function must reset fm.depth_map before
recomputing paths to avoid stale values; in FloydWarshallMap::build, initialize
fm.depth_map to the default "infinite" distance for all pairs (e.g., fill with
f32::INFINITY or a large sentinel) sized to mapsize*mapsize (or call a
reset/clear helper on fm.depth_map), then set the diagonal entries to 0 (using
idx_helper(start_idx, start_idx)) and populate direct exits from
map.get_available_exits as you already do, before running the triple-loop that
updates entries; this ensures removed edges or changed weights don't leave old
shorter distances in depth_map.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d2e5bfac-b2e4-4601-ba3a-19c5499ff6d7

📥 Commits

Reviewing files that changed from the base of the PR and between 34c8d22 and 33ee7be.

📒 Files selected for processing (4)
  • bracket-pathfinding/README.md
  • bracket-pathfinding/examples/floyd_warshall/common.rs
  • bracket-pathfinding/examples/floyd_warshall/main.rs
  • bracket-pathfinding/src/floyd_warshall.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • bracket-pathfinding/README.md
  • bracket-pathfinding/examples/floyd_warshall/main.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: ✅ Check (windows-latest)
  • GitHub Check: 🚨 Lint
  • GitHub Check: 🧪 Test Suite
  • GitHub Check: 🚨 Lint
  • GitHub Check: 🧪 Test Suite
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format all Rust code using cargo fmt --all
Fix all cargo clippy warnings — the CI enforces -D warnings
Add tests for new functionality in the relevant module; for split domains, prefer colocated tests.rs

Files:

  • bracket-pathfinding/src/floyd_warshall.rs
  • bracket-pathfinding/examples/floyd_warshall/common.rs

Comment on lines +41 to +69
pub fn build(fm: &mut FloydWarshallMap, map: &dyn BaseMap) {
let mapsize: usize = fm.size_x * fm.size_y;

for start_idx in 0..mapsize {
let idx = fm.idx_helper(start_idx, start_idx);
fm.depth_map[idx] = 0.;
}

for start_idx in 0..mapsize {
for (end_idx, depth) in map.get_available_exits(start_idx) {
let ste_idx = fm.idx_helper(start_idx, end_idx);
fm.depth_map[ste_idx] = depth;
}
}

for mid_idx in 0..mapsize {
for start_idx in 0..mapsize {
let stm_idx = fm.idx_helper(start_idx, mid_idx);
for end_idx in 0..mapsize {
let ste_idx = fm.idx_helper(start_idx, end_idx);
let mte_idx = fm.idx_helper(mid_idx, end_idx);
let new_depth = fm.depth_map[stm_idx] + fm.depth_map[mte_idx];
let prev_depth = fm.depth_map[ste_idx];

fm.depth_map[ste_idx] = f32::min(new_depth, prev_depth);
}
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset depth_map before rebuilding.

build is public, but it never clears the previous matrix. Rebuilding after exits are removed or weights change will keep stale shorter paths and return wrong distances.

🛠️ Proposed fix
 pub fn build(fm: &mut FloydWarshallMap, map: &dyn BaseMap) {
     let mapsize: usize = fm.size_x * fm.size_y;
+    fm.depth_map.fill(f32::MAX);
 
     for start_idx in 0..mapsize {
         let idx = fm.idx_helper(start_idx, start_idx);
         fm.depth_map[idx] = 0.;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn build(fm: &mut FloydWarshallMap, map: &dyn BaseMap) {
let mapsize: usize = fm.size_x * fm.size_y;
for start_idx in 0..mapsize {
let idx = fm.idx_helper(start_idx, start_idx);
fm.depth_map[idx] = 0.;
}
for start_idx in 0..mapsize {
for (end_idx, depth) in map.get_available_exits(start_idx) {
let ste_idx = fm.idx_helper(start_idx, end_idx);
fm.depth_map[ste_idx] = depth;
}
}
for mid_idx in 0..mapsize {
for start_idx in 0..mapsize {
let stm_idx = fm.idx_helper(start_idx, mid_idx);
for end_idx in 0..mapsize {
let ste_idx = fm.idx_helper(start_idx, end_idx);
let mte_idx = fm.idx_helper(mid_idx, end_idx);
let new_depth = fm.depth_map[stm_idx] + fm.depth_map[mte_idx];
let prev_depth = fm.depth_map[ste_idx];
fm.depth_map[ste_idx] = f32::min(new_depth, prev_depth);
}
}
}
}
pub fn build(fm: &mut FloydWarshallMap, map: &dyn BaseMap) {
let mapsize: usize = fm.size_x * fm.size_y;
fm.depth_map.fill(f32::MAX);
for start_idx in 0..mapsize {
let idx = fm.idx_helper(start_idx, start_idx);
fm.depth_map[idx] = 0.;
}
for start_idx in 0..mapsize {
for (end_idx, depth) in map.get_available_exits(start_idx) {
let ste_idx = fm.idx_helper(start_idx, end_idx);
fm.depth_map[ste_idx] = depth;
}
}
for mid_idx in 0..mapsize {
for start_idx in 0..mapsize {
let stm_idx = fm.idx_helper(start_idx, mid_idx);
for end_idx in 0..mapsize {
let ste_idx = fm.idx_helper(start_idx, end_idx);
let mte_idx = fm.idx_helper(mid_idx, end_idx);
let new_depth = fm.depth_map[stm_idx] + fm.depth_map[mte_idx];
let prev_depth = fm.depth_map[ste_idx];
fm.depth_map[ste_idx] = f32::min(new_depth, prev_depth);
}
}
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bracket-pathfinding/src/floyd_warshall.rs` around lines 41 - 69, The build
function must reset fm.depth_map before recomputing paths to avoid stale values;
in FloydWarshallMap::build, initialize fm.depth_map to the default "infinite"
distance for all pairs (e.g., fill with f32::INFINITY or a large sentinel) sized
to mapsize*mapsize (or call a reset/clear helper on fm.depth_map), then set the
diagonal entries to 0 (using idx_helper(start_idx, start_idx)) and populate
direct exits from map.get_available_exits as you already do, before running the
triple-loop that updates entries; this ensures removed edges or changed weights
don't leave old shorter distances in depth_map.

Comment on lines +74 to +96
pub fn find_lowest_exit(
fm: &FloydWarshallMap,
position: usize,
map: &dyn BaseMap,
) -> Option<usize> {
let exits = map.get_available_exits(position);

if exits.is_empty() {
return None;
}

let mut lowest_depth = fm.max_depth;
let mut lowest_exit = 0;

for (exit, _) in exits {
let pos = fm.idx_helper(position, exit);
if lowest_depth > fm.depth_map[pos] {
lowest_exit = exit;
lowest_depth = fm.depth_map[pos]
}
}

Some(lowest_exit)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

These exit helpers ignore the destination node.

Both helpers rank candidates with depth_map[position, exit], which is only the cost from the current tile to that neighbor. That picks the cheapest/most expensive immediate edge, not the step that moves toward or away from a chosen node as the docs claim. The API needs a target/source index and should compare each candidate exit against that node instead.

🛠️ Direction of fix
 pub fn find_lowest_exit(
     fm: &FloydWarshallMap,
     position: usize,
+    target: usize,
     map: &dyn BaseMap,
 ) -> Option<usize> {
     ...
     for (exit, _) in exits {
-        let pos = fm.idx_helper(position, exit);
+        let pos = fm.idx_helper(exit, target);
         ...
     }
 }

Apply the same signature/lookup change to find_highest_exit, and add a regression test with asymmetric edge weights so the helper must choose based on the destination rather than the local edge cost.

Also applies to: 103-125

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bracket-pathfinding/src/floyd_warshall.rs` around lines 74 - 96, The helpers
currently pick exits by local edge cost; update both find_lowest_exit and
find_highest_exit to accept an additional target/source index parameter (e.g.,
dest: usize) and use fm.idx_helper(exit, dest) to look up the distance in
fm.depth_map when ranking candidates instead of fm.idx_helper(position, exit).
Specifically, change signatures of find_lowest_exit and find_highest_exit to
include the destination/source, iterate exits from
map.get_available_exits(position), compute pos = fm.idx_helper(exit, dest) and
compare fm.depth_map[pos] to choose the min/max exit, and add a regression test
with asymmetric edge weights exercising both helpers so the choice depends on
distance-to-target rather than the immediate edge cost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bracket-pathfinding에 Floyd-Warshall 알고리즘 구현하기

1 participant